Added fixes to properly align paragraphs when loading RTL documents#2352
Added fixes to properly align paragraphs when loading RTL documents#2352claudiu-ior wants to merge 4 commits intosuperdoc-dev:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36824e438c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
caio-pizzol
left a comment
There was a problem hiding this comment.
@claudiu-ior nice work — alignment handling and the start/end flipping are well done and well tested.
one bug: the tab-stop positioning path sets style.right using values measured from the left edge, so RTL paragraphs with tabs will look wrong. left an inline comment with details.
one small cleanup suggestion on a repeated check.
tests: the alignment logic in pm-adapter is well covered. the renderer side (text-align branches, RTL positioning) could use a few tests — the existing createDomPainter setup in index.test.ts would work for that.
caio-pizzol
left a comment
There was a problem hiding this comment.
@claudiu-ior the style.right bug and text-align fixes look good.
heads up: indentation on RTL paragraphs will likely show on the wrong side. the renderer uses paddingLeft/paddingRight which don't flip when dir="rtl" is set, so the indent stays on the left when it should be on the right. there are already helpers for this in bidi.ts (mirrorIndentForRtl) that just need to be wired up — doesn't have to be in this PR, but good to know.
also, the RTL logic is now split across two places in renderer.ts. worth extracting into a feature module (features/rtl-paragraph/) following the pattern from #2324 — makes it easier to add the remaining pieces later.
left a couple inline comments.
Fix RTL (right-to-left) document rendering
Summary
rightToLeft(bidi) paragraph property throughpm-adapterintoParagraphAttrsso the layout engine and DomPainter know a paragraph is RTLnormalizeAlignmentdirection-aware:start/endnow resolve to physicalleft/rightbased on paragraph direction instead of always mapping to LTR defaultslowKashida,mediumKashida,highKashida) which were previously silently droppeddir="rtl"on RTL paragraph and line elements, default theirtext-aligntoright, and position segments from the right edge in segment-based rendering (lines with tabs and explicit X coordinates)Problem
Opening an RTL Arabic document rendered all text left-aligned. Three independent issues contributed:
RTL flag not propagated --
computeParagraphAttrsinpm-adapterreadresolvedParagraphProperties.rightToLeftbut never setdirectionorrtlon the outputParagraphAttrs, so DomPainter had no way to know a paragraph was RTL.Logical-to-physical alignment was not direction-aware --
normalizeAlignmentalways mappedstarttoleftandendtoright, which is wrong for RTL wherestartmeansright. Additionally, Arabic-specific OOXML justify values (lowKashida,mediumKashida,highKashida) were unrecognized and fell through toundefined.DomPainter assumed LTR -- No
dir="rtl"attribute was set on elements. Justified text always usedtext-align: leftas the base (incorrect for RTL, where the last line should right-align). Segment-based rendering (triggered by tabs or explicit positioning) always usedstyle.leftfor X coordinates instead ofstyle.rightfor RTL paragraphs.Changes
pm-adapter/src/attributes/paragraph.tsisRtlfromresolvedParagraphProperties.rightToLeft; pass it tonormalizeAlignment; spreaddirection: 'rtl'andrtl: trueonto output attrspm-adapter/src/attributes/spacing-indent.tsisRtlparameter tonormalizeAlignment; flipstart/endmapping for RTL; addlowKashida/mediumKashida/highKashidaasjustifyaliasespainters/dom/src/renderer.tsdir="rtl"on paragraph and line elements; default RTL text-align toright; userightfor justify base in RTL; introducesetHorizontalPoshelper for segment-based rendering to usestyle.rightinstead ofstyle.leftfor RTLpm-adapter/src/attributes/spacing-indent.test.tsstart/endmapping and kashida variantspm-adapter/src/index.test.tsstart/endflippingTest plan
pnpm test--spacing-indent.test.tsandindex.test.tscover all new logic)intalio_template_17529.docx) -- text should be right-aligned with correct justification